fix(subagent): restore live progress in the await panel - #4757
Conversation
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — bound to exact PR state
Head: e03b7de
Base: dev at origin/dev dadaf19
Merge-base: dadaf19
Frozen diff sha256: 016af1ee886d5fd60cf5c262663e010b3d102a93477746ccda5f334219f6704d
PR body needs-human digest: ff1802fc20de003c7fcbdab2df57281b42cc5c9ec747aa93c7157d72a2588e7d
Blockers:
-
P1 privacy/public-boundary leak.
#liveProgressFieldsrestores rawAgentProgressintoresult.details, but this is not renderer-private. The agent loop copies details intoToolResultMessage.details(packages/agent/src/agent-loop.ts:3515-3534), ACP exposes the result asrawOutput(packages/coding-agent/src/modes/acp/acp-event-mapper.ts:179-211), and full-input telemetry serializes tool-result details verbatim (packages/agent/src/telemetry.ts:932-959).AgentProgresscontains model deltas, tool arguments, arbitrary recent/extracted output, nested task data, and retry diagnostics. The new test proves only that one marker is absent fromresult.content; it also proves the marker is present inresult.details, with no negative assertion across public serialization/ACP/telemetry. Fix by keeping raw progress out of public envelopes or introducing a bounded/redacted approved DTO, then add serialization/ACP/telemetry regression tests. -
P1 missing production render proof.
subagent-live-progress.test.tschecks the execute result's details and content, whilesubagent-render.test.tsrenders synthetic snapshots. No test passes the real execute result tosubagentToolRenderer.renderResult, so the claimed panel restoration is not proven against the production data path. Add an execute-to-renderer assertion for current-tool/recent-output rendering, plus the false-availability stale guard and retry path. -
P2 live retry transition coverage. Retry fields are covered by pure signature/static renderer fixtures, but no live await callback test proves retry start, recovery/clear, or countdown-only suppression on the production update path. Add that coverage after the boundary is fixed.
Evidence run on this exact head:
bun test packages/coding-agent/test/tools/subagent-live-progress.test.ts: 20 pass, 0 fail.- Prior
subagent.test.ts+subagent-render.test.ts: 80 pass, 0 fail. - Base
origin/devfocused suite: 14 pass, 5 fail. - Negative control with the restored field removed: 14 pass, 6 fail.
bun --cwd=packages/coding-agent run check: passed.- Dev CI currently has
Local public surfacessuccessful butPR contract bootstrapfailing; merge state is UNSTABLE.
Do not merge this exact head. Re-review must bind to the new head and refreshed base/digest after the privacy boundary and execute-to-renderer proof are fixed.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
APPROVED
Summary
This restores canonical live subagent progress to the local subagent await panel while retaining the privacy boundary: progress remains in structured UI details and is not included in model-visible receipt content. The change is narrowly scoped and reverses the regression introduced by #4748.
Findings / Required Changes
No required changes. The initially reported potential disclosure concern was independently adjudicated: the cited privacy test uses a synthesized record without a canonical live subagent, so it remains unaffected. The restored payload is consumed by the local renderer and does not enter tool-result text.
CI / Verification
- Reviewed CI for exact head
e03b7de695a7bed10bbcb98048cdec8d1bb7b7b1. - The affected-path test shard, coding-agent and natives checks, TypeScript build, native build, Darwin arm64 smoke, evidence producer, and final affected-path validation passed.
- The two
needs-humanPR-contract failures were intentionally ignored; virtual integration validation remains queued without an execution failure. - No PR code was run locally as part of this review.
Axis Coverage
| Axis | Verdict | Coverage |
|---|---|---|
| A1. Intent / Policy / Contract | APPROVED | The narrow restoration matches the PR intent; no public API or compatibility contract changed. |
| A2. Architecture / Correctness / Failure | APPROVED | The existing live-progress gating, lifecycle, and model-content separation remain intact. |
| A3. Security / Privacy / Trust | APPROVED | Independent adjudication confirmed progress stays in local UI details and does not cross the model-visible boundary. |
| A4. Verification / Tests / CI | APPROVED | Relevant regression coverage and exact-head CI evidence passed; needs-human checks excluded. |
| A5. Context / Compatibility / Platform | APPROVED | Callers, renderer integration, changelog, generated declaration, and platform/package surfaces remain compatible. |
5b8c6ea to
8407322
Compare
The removal therefore fixed no leak and cost two things: the live progress panel went blank, and the emit signature canonicalized progress to a constant null so it could no longer detect real changes. dev has been red since, with subagent-live-progress.test.ts at 14 pass / 5 fail. I approved #4748 without checking that the field reached a model-visible surface, and without running the suite named after the behavior it removed. Confidence: high Scope-risk: narrow Tested: subagent-live-progress 20 pass; the three suites #4748 did run stay at 100 pass; negative control re-removing the field reproduces the exact five dev failures plus the new boundary pin Not-tested: no end-to-end render assertion that the panel paints the restored progress
Raw AgentProgress in await details entered public tool-result, ACP, and telemetry envelopes. Keep the renderer contract live while exposing only tool names, output counts, fast mode, and retry state. Lore-id: 4757-live-progress-contract Constraint: raw model deltas, tool arguments, arbitrary output, and nested task payloads must not cross public result boundaries Constraint: await rendering must preserve live tool and retry transitions Rejected: renderer-only raw AgentProgress | details are serialized and logged outside the TUI Confidence: high Scope-risk: medium Reversibility: revert-commit Tested: focused live-progress and renderer suites; prior subagent suites; coding-agent check Not-tested: Darwin/Windows native CI
8407322 to
64f7c3d
Compare
|
Fix-forward is rebased onto current |
|
@snowykr The prior approval |
Superseded by rebased exact-head review; this review is bound to e03b7de.
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
APPROVE — exact current head after fresh adversarial review
Head: 64f7c3d
Base: dev at 861b000
Merge-base: 861b000
Frozen CI diff sha256: dabb92869f69df173426709798f8f1b063494934d7bf12c19faf64eb29d22916
Fresh architect review: CLEAR / APPROVE. The raw AgentProgress boundary is replaced by an explicit SubagentLiveProgress whitelist.
Fresh executor QA/red-team: passed. Tool-result JSON, ACP rawOutput, full-input telemetry, production execute-to-renderer, stale suppression, retry transitions, and negative controls are covered.
Local evidence: 121 focused/prior assertions passed; bun --cwd=packages/coding-agent run check passed; current dev reproduces 14 pass / 5 fail; removing the approved attachment reproduces 14 pass / 7 fail.
Independent owner approval is bound to this exact commit. The remaining contract check is expected to rerun from needs-human to merge-approved against this exact digest.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
3 participants
What
Restores live subagent progress in the
subagent awaitpanel without returning rawAgentProgressthrough public result surfaces. The fix-forward restores the dev regression through a bounded approved DTO.Why
#4748 removed the
progressfield from#liveProgressFields, which blanked the panel and froze await emit signatures. RawAgentProgresswas not safe to restore directly:detailsis copied into tool-result messages, ACPrawOutput, and full-input telemetry.The await path now converts raw progress into
SubagentLiveProgress, containing only:Model-visible
contentremains the boundedawaitProgressSummary()text containing counts, duration, and ids. Public tool-result, ACP, and telemetry serialization therefore cannot receive model deltas, tool arguments, arbitrary output, or nested task payloads from live progress.Testing
Exact current refs:
64f7c3d1d441913f78726c31a83d695214580010dev/origin/devat861b000cda871100f74ea58b0d5561e0928c5fba861b000cda871100f74ea58b0d5561e0928c5fbadabb92869f69df173426709798f8f1b063494934d7bf12c19faf64eb29d22916ff1802fc20de003c7fcbdab2df57281b42cc5c9ec747aa93c7157d72a2588e7dExecuted on the current rebased head:
bun test packages/coding-agent/test/tools/subagent-live-progress.test.ts packages/coding-agent/test/tools/subagent-render.test.ts packages/coding-agent/test/tools/subagent.test.ts packages/coding-agent/test/task/executor-warnings.test.ts— 121 pass, 0 fail;bun --cwd=packages/coding-agent run check— clean;origin/devfocused negative control — 14 pass, 5 fail;The focused boundary coverage exercises actual tool-result JSON serialization, ACP
rawOutput, full-input telemetry capture, production execute-result tosubagentToolRenderer.renderResult, staleliveProgressAvailable: falsesuppression, approved current-tool/output summary rendering, and live retry start/recovery with countdown-only suppression.Dev CI virtual integration, native build, Darwin arm64 smoke, Windows daemon safety, affected focused suites, and package checks passed in run
32346820748.GJC verdict
dev## [Unreleased]Terminal evidence
2026-08-20T08:46:13Zwith merge commitf52d7eeaa79ac24860401db9ea189c70aa98b7e4.origin/devis verified atf52d7eeaa79ac24860401db9ea189c70aa98b7e4, with PR head64f7c3d1d441913f78726c31a83d695214580010as the second parent.bun run buildpassed on the mergedorigin/devtree.—
[repo owner's gaebal-gajae (clawdbot) 🦞]